Fix up core count detection logic on android - #131234
Conversation
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
|
Tagging subscribers to this area: @JulieLeeMSFT, @VSadov |
There was a problem hiding this comment.
Pull request overview
This PR updates Mono’s Android CPU core-count detection to correctly parse the kernel’s /sys/devices/system/cpu/present “cpulist” format (including non-contiguous ranges), improving the accuracy of mono_cpu_count() on Android.
Changes:
- Parse
/sys/devices/system/cpu/presentas a comma-separated cpulist (single indices and ranges) rather than assuming a single0-(n-1)range. - Increase the read buffer size and add parsing success gating so malformed/partial reads fall back to existing platform logic.
- Clarify the comment describing the
/sysentry format with concrete examples.
|
Do we need the same logic for CoreCLR/NAOT on Android? |
|
judging from my long comment a few lines below the changed lines it sounds like we should be fine with _SC_NPROCESSORS_CONF which is already used by coreclr from what I can see. though I see some inconsistency there, gcenv.unix.cpp and sysinfo.cpp use _SC_NPROCESSORS_CONF on arm/arm64/loongarch64/riscv64 and _SC_NPROCESSORS_ONLN on other architectures |
|
Currently mono on android tries to read |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
src/native/minipal/cpucount.c:49
parse_cpulist_fileshould validate that parsed CPU indices are non-negative and should avoid signed-overflow UB when accumulatingcount. As written, negative indices (or very large ranges) could producemaxIndex == -1or overflowcount, which then flows into CPU set sizing and other CPU-count logic.
if (hi < lo)
{
// Invalid range, discard
count = 0;
break;
}
count += hi - lo + 1;
if (hi > maxIndex)
{
maxIndex = hi;
}
|
/azp run runtime-androidemulator, runtime-android |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
Parse
/sys/devices/system/cpu/presentas a comma-separated cpulist (single indices and ranges) rather than assuming a single 0-(n-1) range.Closes #126417.